WIP: GPU basic rendering support, renderer lib #517

lpenguin · 2021-12-09T06:12:18Z

Preparing for `vange-rs` integration

renderer - a separate library subproject for 2D and 3D rendering

See: Vangers/lib/renderer.

Included libraries in the source tree

See Vangers/lib/renderer/src/renderer/lib:

sokol_gfx - a single-header 3D-API wrapper
HandmadeMath - a single-header math library

Additional dependences:

OpenGL

Main classes

renderer::core::AbstractCoreRenderer: SDL_Renderer replacement
renderer::core::SokolCoreRenderer: SDL_Renderer replacement implementation with sokol_gfx lib
renderer::scene::AbstractRenderer: a class for future GPU rendering implementations
renderer::scene::SokolRenderer: a simple GPU renderer with sokol_gfx lib
renderer::ResourceId<T> a resource identifier which is basically a unique integral type. Each resource type mush typedef its own ResourceId. See renderer::core::Texture resource identifier as an example.
renderer::core::Texture: a texture identifier
renderer::scene::HeightMap: a height map identifier
renderer::scene::util::MapUpdater: an utility class for HeightMap updates

xgraph changes

Since the SDL_Renderer is not working with OpenGL in parallel, I decided to create our own 2D rendering mini-framework and replace SDL_Renderer and SDL_Texture with renderer::core::AbstractCoreRenderer and renderer::core::Texture. Only theese two classes were replaced, we still use SDL_Surface for loading the BMP and pixel rendering.

Original frame rendering changes (iGameMap, vrtMap)

Nothing much was changed: the software rendering into the lineTcolor is still here, but the actual screen rendering was replaced with the new AbstractRendering class. I didn't disable the software renderer completely, because it's hard to isolate and I decided to focus on vange-rs integration at first.

Known problems

Naming - the renderer word is not perfect, because it may conflict with variable names or headers. Consider more unique name like xrender?
Naming - the namespace scheme is not perfect, the renderer::core namespace is confusing - it should be probably used for core types, so we should move 2D rendering classes to renderer::2d namespace or something
The current unique integral type implementation renderer::ResourceId<T> is ugly, because it uses temporary enums for declaring the new type:
```
template<typename T>
struct ResourceId{ int32_t id; }; // omitted the member functions and consts for simplicity 

enum class _HeightMapT; // temporary enum class
typedef ResourceId<_HeightMapT> HeightMap; // uhhh
```
We should consider a simpler approach. Like declaring each type explicitly: struct HeightMap {int32_t id;} or use ResourceId as base class without templates.
Included sokol_gfx and HandmadeMath libraries directly into the code. We could get rid of HandmadeMath and use our own 3d_math.h library. Need to decide how ship or install sokol_gfx.
renderer::scene::SokolRenderer uses full texture updates instead of partial ones - the sokol_gfx library doesn't support partial updates unfortunately. It should not be a big problem since this class was created as dummy class, and the real GPU renderer should be vange-rs renderer.

The AbstractRenderer::render function doesn't support camera rotation and seems to be bloated with too many arguments. I suggest we could create the Camera resource type and pass it to the AbstractRenderer::render

Camera camera_create_perspective(float FOV, float min_dist, float max_dist);
 // Transform is vec3 position and Quaternion rotation. I'm not sure how we implement this exactly
void camera_set_transform(Camera camera, Transform transform);
void camera_destroy(Camera camera);

void render(int viewport_width, int viewport_height, Camera camera);

caiiiycuk · 2021-12-13T04:10:40Z

+1 for renaming core namespace to 2d. Also little bit confusing about file layout, I prefer to group render files based on used library not by render type, so I mean put everything related to sokol under sokol root directory:

sokol/Sokol2dRenderer*
sokol/SokolRenderer*

Moreover I think we should try to move SokolRenderer as 3rd plugin library (so/dll), because this helps to understand which parts are still coupled with legacy renderer, and solve problem with dependencies on sokol-gfx, handmath. In current version, Sokol[2D|Abstract]Renderer is hard coupled with Soko[|3D]Renderer, you can't use Sokol3DRenderer without Sokol2DRenderer because sokolg-gfx initialization happens in Sokol2DRenderer.

So, probably we should remove Sokol2DRenderer from game core, and replace it with Dummy2DRenderer or SDL_renderer as before (?). When you load the renderer (so/dll) library you will get two pointers: to 2DRenderer (can be null), 3DRenderer and apply them to game. If library does not implement 2DRenderer (rust case) then Dummy/SDL_renderer is applied in other case 2d renderer from library will work.

I am not sure should we support mixing renderers like Sokol2D + Rust3D, I think we can assume that rendering library must provide 3d renderer and optionally can provide 2d renderer. So, in game core we will have hardcoded 2D renderer and no 3d renderers at all (only dummy one), this will make our live easier and we can no worry about mixing 3rd party libraries.

Another case that not covered by this PR is a switching between worlds, if we agreed that rust library will load resources by it's own then we need to notify renderer about which world is loaded, and probably we should wait until library says that world is ready.

I will continue to look PR next days, great work!

--
Also DummyRender is under renderer::scene::dummy, while Sokol is under renderer::scene.

kvark

Thank you for doing this grand rendering abstraction, and sorry for getting late to review this!
I believe most if not all of my notes are not important. The principle of abstraction makes sense, and we shouldn't hold it in the air until it's perfect. Instead, we should strike for iterating on it rapidly.

What do you see being the next step? Just trying to hook up vange-rs renderer and see if there are any roadblocks?

kvark · 2021-12-13T04:07:08Z

lib/renderer/src/renderer/ResourceStorage.h

+#include "exception.h"
+
+namespace renderer {
+	template<typename TResourceId, typename TResource>


why is this TResourceId at all? Wouldn't it be the same as Resource<TResource>?

Yes, it's like that because I'm not totally happy with my current unique integral type approach. By unique integral type I mean that we have integer IDs, but we should not pass a Texture ID to the HeightMap methods for example. So I keep the ResourceStorage accepting any struct with int32_t id field in case if I change the current approach.

We can cross this bridge when we get there. Seems strange to overcomplicate the API for something we don't know, at this point. Your choice though, not a big deal.

kvark · 2021-12-13T04:09:06Z

lib/renderer/src/renderer/ResourceStorage.h

+			return it;
+		}
+
+		std::unordered_map<TResourceId, std::unique_ptr<TResource>, ResourceIdHash, ResourceIdEquals> _storage;


the unordered map is probably suboptimal here, and something like a freelist would turn out better
but I doubt it matters right now, nothing to worry about

But would it be way less optimal for id lookup? I mean the idea of the ResourceStorage is to map resource id to the internal resource representation. For example if we have a Light resource and change the light position every frame, and the concrete implementation maps the light id to it's internal light object and updates the position

yes, a freelist would mean that ID lookup is just indexing into an array. No hashing, bucketing, etc, involved.

Ah, yes, exactly. I thought it's based on linked list

lib/renderer/src/renderer/core/AbstractCoreRenderer.h

lib/renderer/src/renderer/core/sokol/SokolTexture.cpp

kvark · 2021-12-13T04:17:49Z

lib/renderer/src/renderer/core/sokol/SokolTexture.cpp

+	render_context->bind.vertex_buffers[0] = vbuf;
+	render_context->vertex_buffer = vbuf;
+
+	uint16_t indices[] = {


if we are just drawing 2 triangles, can't we avoid index buffer altogether an just have a triangle strip with 4 vertices?

kvark · 2021-12-13T04:18:19Z

lib/renderer/src/renderer/core/sokol/SokolTexture.cpp

+	std::unique_ptr<RenderContext> render_context = std::make_unique<RenderContext>(RenderContext{});
+
+	float vertices[] = {
+		-1.0f,  1.0f, 0.0f, 0.0f,


nit: we shouldn't really need this vertex buffer, the values can be produced from within the shader based on gl_VertexID

kvark · 2021-12-13T04:20:22Z

lib/renderer/src/renderer/scene/AbstractRenderer.h

+		virtual HeightMap map_create(const MapDescription& map_description) = 0;
+		virtual void map_destroy(HeightMap map) = 0;
+		virtual void map_query(HeightMap map, int32_t* width, int32_t* height) = 0;
+		virtual void map_update_data(HeightMap map, const Rect& rect, uint8_t* height, uint8_t* meta) = 0;


where is the data to update with?

The idea is that we copy a slice of data from the lineT to the height and meta and pass them to the map_update_data. And delete them after the call. So may be the arguments should be const uint8_t*. I wrote some comments to the AbstractRenderer.h, see my last commits and MapUpdater class

ok, yes, these should be const uint8_t* then

kvark · 2021-12-13T04:25:09Z

lib/renderer/src/renderer/scene/sokol/HeightMap.cpp

+}
+
+HeightMap::~HeightMap() {
+	delete[] height_map;


should also delete the meta and palette

lpenguin · 2021-12-13T10:46:18Z

Thanks for the review, guys!

@caiiiycuk

Moreover I think we should try to move SokolRenderer as 3rd plugin library (so/dll)

Yes, that make sense

+1 for renaming core namespace to 2d.

Ok, I'll think about the new naming scheme. Any ideas of the better name for the renderer library? I mean the current one is too broad :)

So, probably we should remove Sokol2DRenderer from game core, and replace it with Dummy2DRenderer or SDL_renderer as before (?). When you load the renderer (so/dll) library you will get two pointers: to 2DRenderer (can be null), 3DRenderer and apply them to game. If library does not implement 2DRenderer (rust case) then Dummy/SDL_renderer is applied in other case 2d renderer from library will work.

I am not sure should we support mixing renderers like Sokol2D + Rust3D, I think we can assume that rendering library must provide 3d renderer and optionally can provide 2d renderer. So, in game core we will have hardcoded 2D renderer and no 3d renderers at all (only dummy one), this will make our live easier and we can no worry about mixing 3rd party libraries.

My main motivation of having the Sokol 2D renderer is that we cannot mix SDL_renderer and direct OpenGL calls (from SDL_GL_CreateContext). I have not found any information of using them in parallel (except of a weird SDL_GL_BindTexture call). I suppose we cannot just use SDL_renderer and vange-rs renderer in parallel, for example. I guess SDL developers don't want anyone to mess with their internal SDL_Renderer context. So I decided that we should have our own OpenGL 2D renderer.

My initial idea was that we always have Sokol 2D render as SDL_Renderer replacement and just pass a pointer of the OpenGL context to the concrete Abstract[Scene]Renderer implementation (to the vange-rs lib for example). The only problem here is that the concrete implementation may unintentionally mess with the internal Sokol OpenGL state. Uh, and sokol_gfx doesn't have a way to pass the OpenGL context to it, so the Sokol[Scene]Render just assumes that everything is already initialized..

I like your suggestion that every renderer implementation should provide a scene renderer and a 2D renderer. That could be another solution of the SDL_renderer/OpenGL problem.

So I see a few possible options:

I'm wrong about using the SDL_renderer and OpenGL in parallel. I wish I could find a working example of that.
We permanently replace SDL_renderer with Sokol 2D renderer and pass it's opengl context to the Rust renderer. The OpenGL context comes from SDL_GL_CreateContext call.
We force Abstract[Scene]Renderer to implement both 2D and scene renderers. In that case we should implement 2D rendering on the Rust side
Any other option?

Another case that not covered by this PR is a switching between worlds, if we agreed that rust library will load resources by it's own then we need to notify renderer about which world is loaded, and probably we should wait until library says that world is ready.

This is an important one: the current approach is to pass a world information from Vangers to the Abstract[Scene]Renderer implementation. See

virtual void renderer::scene::AbstractRenderer::map_update_data(HeightMap map, const Rect& rect, uint8_t* height, uint8_t* meta) = 0;

This method updates a part of the height map with new height and meta information. The idea is that we have an empty height map at the start and update it during the game. The updates will be triggered if the part of the level was loaded from the disk or there was an update in the game world, like roof collapse. And we do not have multiple updates per frame and accumulating them during the frame instead (basically expanding the update region). See MapUpdater class

I plan to update every renderer object from the game, like 3D models or sprites. @kvark can we support such approach on the Rust side?

@kvark Thanks for your notes :)

The principle of abstraction makes sense, and we shouldn't hold it in the air until it's perfect. Instead, we should strike for iterating on it rapidly.

Totally agreed

What do you see being the next step? Just trying to hook up vange-rs renderer and see if there are any roadblocks?

Yes, I think we should focus on the rendering of the height map in game using the vange-rs library. So we should decide what would be an interface of the vange-rs library and probably adapt the AbstractRenderer interface to it.

lpenguin · 2021-12-30T07:00:42Z

Updates:

Removed HeightMap and Camera resource handles, since they are always single instance
Added a stub for vange-rs implementation: 91882b2
One can set a renderer instance here:

Vangers/src/road.cpp

Line 381 in 40c518e

renderer::scene::RenderingContext::create(std::make_unique<renderer::scene::SokolRenderer>());

kvark

Thank you for making those Rusty changes!

lib/renderer/src/renderer/lib/sokol_gfx.h

lib/renderer/src/renderer/scene/rust/vange_rs.h

caiiiycuk · 2021-12-31T13:41:58Z

lib/renderer/src/renderer/ResourceStorage.h

+		typedef typename std::unordered_map<TResourceId, std::unique_ptr<TResource>, ResourceIdHash, ResourceIdEquals>::iterator iterator;
+
+		ResourceStorage()
+			:_next(0)


uses convention that resource id is valid if >= 0, it's not clear thought

I think we should never use the internal id field explicitly, so zero should be fine. Anyway I plan to refactor the "resource" system, since it's a bit over engineered..

caiiiycuk · 2021-12-31T13:44:38Z

lib/renderer/src/renderer/core/AbstractCoreRenderer.h

+	};
+
+	enum class BlendMode {
+		None,


not clear the meaning of None, do not blend at all? or copy?

It means Copy, yes. Should it be called "Copy"? What's the usual naming sheme?

caiiiycuk · 2021-12-31T13:57:11Z

lib/renderer/src/renderer/scene/AbstractRenderer.h

+		// Destroys the HeightMap
+		virtual void map_destroy() = 0;
+
+		// Updates a rectangular area the selected HeightMap with height data from *height* and meta data from *meta*


didn't get it, about which pointers this comment

It's an old comment, thanks. We use now lineT from MapDescription.

caiiiycuk · 2021-12-31T13:58:33Z

lib/renderer/src/renderer/scene/rust/vange_rs.h

@@ -0,0 +1,82 @@
+#ifndef VANGE_RS_H


what about extern "C" for all functions?

But there is already extern "C", see #ifdef __cplusplus

caiiiycuk · 2021-12-31T13:59:53Z

lib/renderer/src/renderer/scene/sokol/MapUpdater.cpp

+
+		uint8_t* _lineT_r = lineT[iy + y_start];
+		if(_lineT_r != nullptr){
+			std::memcpy(height_map_r, _lineT_r + x_start, sizeof(uint8_t) * width);


why we need to do memcpy, why not to pass original pointers?

I think one shouldn't use SokolRenderer now in general, because it was used only as simplified version for Rust renderer, like a stub before we've got the Rust renderer.

The MapUpdater used an old approach, where I passed the map updates via region copies

caiiiycuk · 2022-01-03T06:16:23Z

src/CMakeLists.txt

 	${WIN_LIB}
 	${STEAM_LIBS}
 	xtool
 	xgraph
 	xsound
+	dl


Is it really needed? We use static linkage (at least in rust part I seen staticlib crate)

I should check this, but I remember that I get symbol not found on linux without -ldl

lpenguin · 2022-01-10T20:57:59Z

Added vange-rs renderer via Rust FFI.

How to build:

install rustup
clone https://github.com/kvark/vange-rs
build libvangers_ffi.a

cd vange-rs/lib/ffi
cargo build

libvangers_ffi.a should be in the vange-rs/target/debug dir
copy vange-rs/res folder to vangers data dir
build Vangers with -DRUST_VANGERS_ROOT=<sources root>/vange-rs/target/debug.

Controls

One can switch between old and new Rust renderer with \ button

lib/renderer/src/renderer/scene/rust/vange_rs.h

lib/renderer/src/renderer/scene/rust/RustRenderer.cpp

lpenguin · 2022-01-18T15:31:51Z

Renamed renderer::core::AbstractCoreRenderer to renderer::compositor::AbstractCompositor
Renamed renderer::scene::AbstractRenderer to renderer::visualbackend::AbstractVisualBackend
Removed sokol_gfx related rendering stuff (SokolRenderer, SokolCoreRenderer)
rv_resize support
rv_api_1 support

lpenguin · 2022-01-18T15:37:32Z

lib/renderer/src/renderer/visualbackend/rust/vange_rs.h

+
+	void rv_map_update_data(rv_context context, rv_rect region);
+
+	void rv_map_update_palette(rv_context context, int32_t first_entry, int32_t entry_count, uint8_t* palette);


I'm thinking about passing uint8_t** lineT to rv_map_update_data, and remove it from MapDescription. This way, the Rust side wouldn't keep the lineT pointer permanently. @kvark what do you think?

yep, that is fine

lpenguin · 2022-01-18T15:45:26Z

@kvark i'm thinking about new name for the libvangers_ffi library, because the current one is a bit confusing: one can decide that it's related to C++ Vangers. Should we rename it to something like librust_vangers_ffi or libvange_rs_ffi?

kvark · 2022-01-18T19:06:55Z

The name seems fine. We could change to something like librusty_vangers or even librv_ffi.

lpenguin · 2022-01-19T08:28:23Z

@kvark librusty_vangers looks good

* AbstractCoreRenderer: SDL_Renderer replacement * AbstractRenderer: class for future GPU rendering implementations Rename

kvark · 2022-01-19T16:32:34Z

@lpenguin renamed in kvark/vange-rs@3d6ab75

lpenguin force-pushed the lp-abs-rend branch 3 times, most recently from 1e00bdd to 0580afd Compare December 11, 2021 17:04

lpenguin changed the base branch from master to alpha December 12, 2021 07:10

kvark reviewed Dec 13, 2021

View reviewed changes

kvark reviewed Dec 31, 2021

View reviewed changes

lib/renderer/src/renderer/lib/sokol_gfx.h Outdated Show resolved Hide resolved

lib/renderer/src/renderer/scene/rust/vange_rs.h Outdated Show resolved Hide resolved

lib/renderer/src/renderer/scene/rust/vange_rs.h Outdated Show resolved Hide resolved

kvark mentioned this pull request Dec 31, 2021

Basic FFI library target, API version 1 kvark/vange-rs#174

Merged

caiiiycuk reviewed Dec 31, 2021

View reviewed changes

lpenguin force-pushed the lp-abs-rend branch from e0ee4bf to 82049d0 Compare January 1, 2022 19:38

caiiiycuk reviewed Jan 3, 2022

View reviewed changes

lpenguin force-pushed the lp-abs-rend branch 2 times, most recently from 20a840d to 619b4a4 Compare January 10, 2022 20:47

lpenguin force-pushed the lp-abs-rend branch from d19efb5 to d43f99c Compare January 16, 2022 20:49

kvark reviewed Jan 17, 2022

View reviewed changes

lib/renderer/src/renderer/scene/rust/vange_rs.h Outdated Show resolved Hide resolved

lib/renderer/src/renderer/scene/rust/RustRenderer.cpp Outdated Show resolved Hide resolved

lpenguin force-pushed the lp-abs-rend branch 2 times, most recently from 6fd1fb9 to ccf2dc2 Compare January 18, 2022 15:23

lpenguin commented Jan 18, 2022

View reviewed changes

renderer lib

a779789

* AbstractCoreRenderer: SDL_Renderer replacement * AbstractRenderer: class for future GPU rendering implementations Rename

lpenguin force-pushed the lp-abs-rend branch from ccf2dc2 to a779789 Compare January 19, 2022 09:42

lpenguin changed the base branch from alpha to renderer-ng January 20, 2022 16:56

stalkerg merged commit f3894d0 into KranX:renderer-ng Jan 25, 2022

kvark mentioned this pull request Jan 29, 2022

Add OpenGL or Vulkan renderer KD-lab-Open-Source/Perimeter#14

Open


		void rv_map_update_data(rv_context context, rv_rect region);

		void rv_map_update_palette(rv_context context, int32_t first_entry, int32_t entry_count, uint8_t* palette);

WIP: GPU basic rendering support, renderer lib #517

WIP: GPU basic rendering support, renderer lib #517

Conversation

lpenguin commented Dec 9, 2021 • edited Loading

Preparing for vange-rs integration

renderer - a separate library subproject for 2D and 3D rendering

Included libraries in the source tree

Main classes

xgraph changes

Original frame rendering changes (iGameMap, vrtMap)

Known problems

caiiiycuk commented Dec 13, 2021 • edited Loading

kvark left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lpenguin Dec 13, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lpenguin commented Dec 13, 2021 • edited Loading

lpenguin commented Dec 30, 2021

kvark left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lpenguin Jan 18, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lpenguin Jan 18, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lpenguin commented Jan 10, 2022 • edited Loading

How to build:

Controls

lpenguin commented Jan 18, 2022 • edited Loading

lpenguin Jan 18, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lpenguin commented Jan 18, 2022 • edited Loading

kvark commented Jan 18, 2022

lpenguin commented Jan 19, 2022

kvark commented Jan 19, 2022

lpenguin commented Dec 9, 2021 •

edited

Loading

Preparing for `vange-rs` integration

caiiiycuk commented Dec 13, 2021 •

edited

Loading

lpenguin Dec 13, 2021 •

edited

Loading

lpenguin commented Dec 13, 2021 •

edited

Loading

lpenguin Jan 18, 2022 •

edited

Loading

lpenguin Jan 18, 2022 •

edited

Loading

lpenguin commented Jan 10, 2022 •

edited

Loading

lpenguin commented Jan 18, 2022 •

edited

Loading

lpenguin Jan 18, 2022 •

edited

Loading

lpenguin commented Jan 18, 2022 •

edited

Loading